-
Notifications
You must be signed in to change notification settings - Fork 936
Replace misspell with cspell and perform spelling fixes #4728 #4754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Replace misspell with cspell and perform spelling fixes #4728 #4754
Conversation
|
An extension to this PR could be to add in https://textlint.github.io/ also similar to otel.io but out of scope. |
trask
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
.cspell.yaml
Outdated
| - name: CodeBlock | ||
| pattern: | | ||
| / | ||
| ^(\s*[~`]{3,}) # code-block start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are ~ code blocks a thing?
| ^(\s*[~`]{3,}) # code-block start | |
| ^(\s*[`]{3,}) # code-block start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the docs they are https://www.markdownguide.org/extended-syntax/#fenced-code-blocks & it is also the regex from otel.io.
.cspell.yaml
Outdated
| ([~`]) | ||
| [^~`]*? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question here
| ([~`]) | |
| [^~`]*? | |
| ([`]) | |
| [^`]*? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above but this is in addition to otel.io
|
|
||
| # Attempt to fix issues / regenerate tables. | ||
| .PHONY: fix | ||
| fix: misspell-correction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does CSPELL have an autocorrect function?
If not I'd prefer not to move to CSPELL. My opinion on lints in general is if you can't get a tool to automatically do it, it's just a hassle for everyone. While misspell may not have the same feature-set of cspell, I don't want to switch until fix works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do agree that cspell in the community repo can sometimes be annoying (maintaining the dictionary).
That said, it did catch a lot of misspellings in this PR!
A couple of thoughts:
- we could avoid running it on PRs and instead have a nightly job that opens an issue if it fails, so any issues can be fixed async (possibly by assigning the issue to copilot)
- we could start running copilot (or other code reviewer) on all PRs which should catch spelling mistakes going forward without relying on cspell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree maintaining the dictionary is a pain point, hopefully we could find a way to sync the core dictionaries to the different repos and have only a small local repo specific dictionary. Also note there is a lot of custom dictionaries we could install to hopefully close that gap.
What I am thinking is if we change the cspell job to not fail on mistakes but instead comment on the files with its findings. There is an issue to add this functionality to cspell using already available interface. We do a nightly full check of all docs as suggested by trask.
To complement this and still offer a fix command, we introduce textlint to correct common misspellings. This linter is also used by otel.io. We could then fail the pipeline if issues arise but isn't a replacement for cspell given the difference in issues flagged.
Fixes #4728
Changes
This makes an attempt to replace misspell with cspell.
Cspell was selected due to:
While working through the switch, corrections to text was made as necessary or added to dictionary. Note i started with the same dictionary as otel.io and extended it in 133810c
Note text within ` code blocks are automatically excluded hence text has been standardised.
For non-trivial changes, follow the change proposal process.
CHANGELOG.mdfile updated for non-trivial changes